Parameters `roles`, `config_entires`, and `pg_hba_rules` to `postgresql::server` for hiera #950

ekohl · 2018-01-18T15:21:34Z

This includes refactoring like using contain rather than the anchor pattern.

The autorequires should ensure the sql is always executed after the server is configured. This was needed because the hiera entries would create a cyclic dependency. For some reason the test is failing so it might not be working.

eputnam · 2018-01-18T17:27:50Z

@ekohl autorequires are not evaluated at compile time so there's no way to test them with unit tests. that_requires does not test autorequire.

ekohl · 2018-01-30T15:57:34Z

@eputnam how should we cover that then? Ignore it in the tests? More unit tests?

ekohl · 2018-01-31T17:01:25Z

lib/puppet/type/postgresql_psql.rb

@@ -119,6 +119,8 @@ def matches(value)
    newvalues(:true, :false)
  end

+  autorequire(:class) { 'postgresql::server::service' }


I don't know if this actually works. I had to add some require => Class['postgresql::server::service'] so it might not.

Turns out it didn't but rspec-puppet was able to test for it. Now updated so it does work.

ekohl · 2018-01-31T17:02:05Z

I believe this should be ready for review. There's an acceptance test that verifies the behavior.

ekohl · 2018-02-13T11:56:21Z

The build now fails with Current textdomain ("master_domain") was not added, use FastGettext.add_text_domain ! but I don't know how to actually solve that. Was that something I changed?

eputnam · 2018-02-13T16:51:10Z

@ekohl nothing you did. all better now!

mmoll · 2018-02-15T22:22:32Z

@ekohl this needs a rebase :)

ekohl · 2018-02-16T11:54:00Z

I did rebase it, but somehow I'm now getting an error that it attempts to create the user before the server is started despite there being an autorequire for this that the unit tests prove exists. Maybe the require -> undef is causing unexpected behavior?

ekohl · 2018-03-07T10:27:13Z

Rebased. I also opened #967 to get some preparation patches merged and reduce the size of this PR.

ehelms · 2018-03-23T12:38:39Z

@ekohl Your cleanup was merged (#967) so I think this can be rebased now?

hunner

I'm not a huge fan of adding hash class parameters to component modules to wrap defines for hiera because it opens the doors to using component defines rather than abstracting away the generic model and using defined types in roles & profiles that fit the specific business use case. But adding it does save a lot of work for people who just want to do it anyway. LGTM

hunner · 2018-03-26T23:10:09Z

spec/unit/classes/server_spec.rb

+    end
+
+    it { is_expected.to compile.with_all_deps }
+    it do # rubocop:disable RSpec/ExampleLength


Rubocop complains about this.

hunner · 2018-03-26T23:12:03Z

spec/unit/defines/server/reassign_owned_by_spec.rb

  it { is_expected.to contain_postgresql__server__reassign_owned_by('test') }

  it {
    is_expected.to contain_postgresql_psql('reassign_owned_by:test:REASSIGN OWNED BY "test_old_role" TO "test_new_role"')
      .with_command('REASSIGN OWNED BY "test_old_role" TO "test_new_role"')
      .with_onlyif(%r{SELECT tablename FROM pg_catalog.pg_tables WHERE\s*schemaname NOT IN \('pg_catalog', 'information_schema'\) AND\s*tableowner = 'test_old_role'.*}m)
-      .that_requires('Class[postgresql::server]')
+      .that_requires('Class[Postgresql::Server::Service]')


Ooo, dependency refs are case insensitive. TIL

Allow adding roles, config entires and hba rules via hiera

hunner · 2018-03-29T20:33:58Z

Whoops, merged this without docs.

ekohl · 2018-03-30T12:44:59Z

Thanks! This will need some monitoring because even though the tests pass, I have seen some cases where the actual dependency isn't enforced. I'm starting to suspect there could be a bug in puppet types.

Add docs for #950

hunner · 2018-04-03T23:26:54Z

spec/acceptance/overridden_settings_spec.rb

+        },
+      },
+      config_entries => {
+        max_connections => 200,


This seems to be causing issues on some of our test nodes:

09:04:41 Apr 03 09:04:39 k8ytf5mtkdzr708 pg_ctl[14776]: FATAL: could not create shared memory segment: Invalid argument 09:04:41 Apr 03 09:04:39 k8ytf5mtkdzr708 pg_ctl[14776]: DETAIL: Failed system call was shmget(key=5432001, size=36954112, 03600). 09:04:41 Apr 03 09:04:39 k8ytf5mtkdzr708 pg_ctl[14776]: HINT: This error usually means that PostgreSQL's request for a shared memory segment exceeded your kernel's SHMMAX parameter. You can either reduce the request size or reconfigure the kernel with larger SHMMAX. To reduce the request size (currently 36954112 bytes), reduce PostgreSQL's shared memory usage, perhaps by reducing shared_buffers or max_connections. 09:04:41 Apr 03 09:04:39 k8ytf5mtkdzr708 pg_ctl[14776]: If the request size is already small, it's possible that it is less than your kernel's SHMMIN parameter, in which case raising the request size or reconfiguring SHMMIN is called for.

Should I lower it to 20?

let's continue this in #974

In d69ff1e I changed the require to be an autorequire. However, even in the PR I noted[1] it may not work. Now I realize this may not work because you can't autorequire on a class. In the same way, you can't collect classes. I started to see this on my Puppet 7 testing (edited for brevity): Notice: /Stage[main]/Postgresql::Client/Package[postgresql-client]/ensure: created Notice: /Stage[main]/Postgresql::Client/File[/usr/local/bin/validate_postgresql_connection.sh]/ensure: defined content as '{sha256}f34c53aa433754c6343e6382cd994e33f2887f9eb54503421eb0db55a55e14a6' Notice: /Stage[main]/Postgresql::Server::Install/Package[postgresql-server]/ensure: created Notice: /Stage[main]/Postgresql::Server::Initdb/File[/var/lib/pgsql/data]/ensure: created Notice: /Stage[main]/Postgresql::Server::Initdb/Exec[postgresql_initdb]/returns: The files belonging to this database system will be owned by user "postgres". Notice: /Stage[main]/Postgresql::Server::Initdb/Exec[postgresql_initdb]/returns: This user must also own the server process. Notice: /Stage[main]/Postgresql::Server::Initdb/Exec[postgresql_initdb]/returns: Notice: /Stage[main]/Postgresql::Server::Initdb/Exec[postgresql_initdb]/returns: initdb: error: invalid locale settings; check LANG and LC_* environment variables Error: '/usr/bin/initdb --pgdata '/var/lib/pgsql/data'' returned 1 instead of one of [0] Error: /Stage[main]/Postgresql::Server::Initdb/Exec[postgresql_initdb]/returns: change from 'notrun' to ['0'] failed: '/usr/bin/initdb --pgdata '/var/lib/pgsql/data'' returned 1 instead of one of [0] Info: Class[Postgresql::Server::Initdb]: Unscheduling all events on Class[Postgresql::Server::Initdb] Notice: /Stage[main]/Postgresql::Server::Config/File[/var/lib/pgsql/data/postgresql.conf]: Dependency Exec[postgresql_initdb] has failures: true Warning: /Stage[main]/Postgresql::Server::Config/File[/var/lib/pgsql/data/postgresql.conf]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/File[systemd-conf-dir]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/File[systemd-override]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/File[old-systemd-override]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Exec[restart-systemd]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_hba.conf]/Concat_file[/var/lib/pgsql/data/pg_hba.conf]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_hba.conf]/File[/var/lib/pgsql/data/pg_hba.conf]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_hba.conf]/Concat_fragment[/var/lib/pgsql/data/pg_hba.conf_header]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Config_entry[port]/Postgresql_conf[port]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Config_entry[data_directory]/Postgresql_conf[data_directory]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_ident.conf]/Concat_file[/var/lib/pgsql/data/pg_ident.conf]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_ident.conf]/File[/var/lib/pgsql/data/pg_ident.conf]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Concat[/var/lib/pgsql/data/pg_ident.conf]/Concat_fragment[/var/lib/pgsql/data/pg_ident.conf_header]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[local access as postgres user]/Concat::Fragment[pg_hba_rule_local access as postgres user]/Concat_fragment[pg_hba_rule_local access as postgres user]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[local access to database with same name]/Concat::Fragment[pg_hba_rule_local access to database with same name]/Concat_fragment[pg_hba_rule_local access to database with same name]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[allow localhost TCP access to postgresql user]/Concat::Fragment[pg_hba_rule_allow localhost TCP access to postgresql user]/Concat_fragment[pg_hba_rule_allow localhost TCP access to postgresql user]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[deny access to postgresql user]/Concat::Fragment[pg_hba_rule_deny access to postgresql user]/Concat_fragment[pg_hba_rule_deny access to postgresql user]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[allow access to all users]/Concat::Fragment[pg_hba_rule_allow access to all users]/Concat_fragment[pg_hba_rule_allow access to all users]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Config/Postgresql::Server::Pg_hba_rule[allow access to ipv6 localhost]/Concat::Fragment[pg_hba_rule_allow access to ipv6 localhost]/Concat_fragment[pg_hba_rule_allow access to ipv6 localhost]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Service/Anchor[postgresql::server::service::begin]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Service/Service[postgresqld]: Skipping because of failed dependencies Info: Class[Postgresql::Server]: Unscheduling all events on Class[Postgresql::Server] Warning: /Stage[main]/Postgresql::Server::Service/Postgresql_conn_validator[validate_service_is_running]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Service/Anchor[postgresql::server::service::end]: Skipping because of failed dependencies Warning: /Stage[main]/Postgresql::Server::Reload/Exec[postgresql_reload]: Skipping because of failed dependencies Info: Class[Foreman::Database::Postgresql]: Scheduling refresh of Postgresql::Server::Db[foreman] Info: Postgresql::Server::Db[foreman]: Scheduling refresh of Postgresql::Server::Database[foreman] Info: Postgresql::Server::Db[foreman]: Scheduling refresh of Postgresql::Server::Role[foreman] Info: Postgresql::Server::Db[foreman]: Scheduling refresh of Postgresql::Server::Database_grant[GRANT foreman - ALL - foreman] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[CREATE ROLE foreman ENCRYPTED PASSWORD ****] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" NOSUPERUSER] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" NOCREATEDB] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" NOCREATEROLE] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" LOGIN] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" INHERIT] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" NOREPLICATION] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE "foreman" CONNECTION LIMIT -1] Info: Postgresql::Server::Role[foreman]: Scheduling refresh of Postgresql_psql[ALTER ROLE foreman ENCRYPTED PASSWORD ****] Info: Postgresql::Server::Database_grant[GRANT foreman - ALL - foreman]: Scheduling refresh of Postgresql::Server::Grant[database:GRANT foreman - ALL - foreman] Error: /Stage[main]/Foreman::Database::Postgresql/Postgresql::Server::Db[foreman]/Postgresql::Server::Role[foreman]/Postgresql_psql[CREATE ROLE foreman ENCRYPTED PASSWORD ****]: Could not evaluate: Error evaluating 'unless' clause, returned pid 1676 exit 2: 'psql: error: could not connect to server: No such file or directory Is the server running locally and accepting connections on Unix domain socket "/var/run/postgresql/.s.PGSQL.5432"? ' As can be seen, various resources are correctly skipped due to failed dependencies but Postgresql_psql isn't. This changes it to an autorequire on the service. This should work since the name of the service resource is static. There is an edge case where the service isn't managed. For that the anchor is also auto-required. This gives less guarantee, but it's a best-effort for those users. [1]: puppetlabs#950 (comment) Fixes: d69ff1e

ekohl mentioned this pull request Jan 18, 2018

Introduce katello::postgresql theforeman/puppet-katello#217

Closed

david22swan added the needs-rebase label Jan 22, 2018

ekohl force-pushed the hiera-defined-types branch from d7b3f1c to 028f121 Compare January 30, 2018 16:04

david22swan removed the needs-rebase label Jan 30, 2018

ekohl force-pushed the hiera-defined-types branch 2 times, most recently from 5144a5b to 34550e2 Compare January 31, 2018 17:00

ekohl commented Jan 31, 2018

View reviewed changes

ekohl force-pushed the hiera-defined-types branch from 34550e2 to 4bba246 Compare January 31, 2018 17:36

ekohl force-pushed the hiera-defined-types branch from 4bba246 to af0b879 Compare February 16, 2018 11:52

david22swan added the needs-rebase label Mar 2, 2018

ekohl mentioned this pull request Mar 7, 2018

Various cleanups #967

Merged

ekohl force-pushed the hiera-defined-types branch from af0b879 to 61a7574 Compare March 7, 2018 10:26

ekohl force-pushed the hiera-defined-types branch from 61a7574 to e9ad1cb Compare March 26, 2018 09:42

hunner reviewed Mar 26, 2018

View reviewed changes

ekohl mentioned this pull request Mar 28, 2018

Resolve dependencies to the databases automatically #963

Closed

ekohl added 3 commits March 28, 2018 17:58

Autorequire the service class for postgresql_psql

d69ff1e

Use contain rather than the anchor pattern

5ae1dd4

Allow adding roles, config entires and hba rules via hiera

27a4813

ekohl force-pushed the hiera-defined-types branch from e9ad1cb to 27a4813 Compare March 28, 2018 16:02

ekohl mentioned this pull request Mar 28, 2018

Fixes #23054 - Chain smart proxies to require the Foreman Service theforeman/puppet-foreman#633

Closed

hunner merged commit 27a4813 into puppetlabs:master Mar 29, 2018

hunner added a commit that referenced this pull request Mar 29, 2018

Merge pull request #950 from ekohl/hiera-defined-types

b0de1ff

Allow adding roles, config entires and hba rules via hiera

hunner added feature needs-docs and removed needs-rebase labels Mar 29, 2018

hunner added a commit to hunner/puppetlabs-postgresql that referenced this pull request Mar 29, 2018

Add docs for puppetlabs#950

aeebd23

ekohl deleted the hiera-defined-types branch March 30, 2018 12:43

clairecadman added a commit that referenced this pull request Apr 3, 2018

Merge pull request #972 from hunner/docs_for_950

624bd9f

Add docs for #950

hunner reviewed Apr 3, 2018

View reviewed changes

hunner changed the title ~~Allow adding roles, config entires and hba rules via hiera~~ Parameters roles, config_entires, and pg_hba_rules to postgresql::server for hiera Apr 5, 2018

mrmcshane mentioned this pull request Sep 24, 2018

Parameters databases to postgresql::server for hiera #1027

Closed

diogokiss pushed a commit to Tradeshift/puppetlabs-postgresql that referenced this pull request Jan 3, 2019

Add docs for puppetlabs#950

5700637

ekohl mentioned this pull request May 27, 2021

(MODULES-8700) Autorequire the service in postgresql_psql #1276

Merged

cegeka-jenkins pushed a commit to cegeka/puppet-postgresql that referenced this pull request Feb 3, 2022

Add docs for puppetlabs#950

19fb9dc

Ramesh7 added the maintenance label Jun 12, 2023

Parameters roles, config_entires, and pg_hba_rules to postgresql::server for hiera #950

Parameters roles, config_entires, and pg_hba_rules to postgresql::server for hiera #950

Uh oh!

Conversation

ekohl commented Jan 18, 2018

Uh oh!

eputnam commented Jan 18, 2018

Uh oh!

ekohl commented Jan 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekohl commented Jan 31, 2018

Uh oh!

ekohl commented Feb 13, 2018

Uh oh!

eputnam commented Feb 13, 2018

Uh oh!

mmoll commented Feb 15, 2018

Uh oh!

ekohl commented Feb 16, 2018

Uh oh!

ekohl commented Mar 7, 2018

Uh oh!

ehelms commented Mar 23, 2018

Uh oh!

hunner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hunner commented Mar 29, 2018

Uh oh!

ekohl commented Mar 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Parameters `roles`, `config_entires`, and `pg_hba_rules` to `postgresql::server` for hiera #950

Parameters `roles`, `config_entires`, and `pg_hba_rules` to `postgresql::server` for hiera #950